Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wallhero add model ACL-401S3I #980

Merged
merged 2 commits into from
Nov 9, 2023
Merged

wallhero add model ACL-401S3I #980

merged 2 commits into from
Nov 9, 2023

Conversation

thinkaName
Copy link
Contributor

@thinkaName thinkaName commented Sep 22, 2023 •
edited
Loading

Manufacturers wallhero add new model ACL-401S3I zigbee-switch.
Please review it.

liuyd96
liuyd96 reviewed Oct 17, 2023
View reviewed changes
drivers/SmartThings/zigbee-switch/src/wallhero/init.lua Outdated
@@ -21,7 +21,8 @@ local Scenes = zcl_clusters.Scenes

local FINGERPRINTS = {
{ mfr = "WALL HERO", model = "ACL-401S4I", switches = 4, buttons = 0 },
{ mfr = "WALL HERO", model = "ACL-401S8I", switches = 4, buttons = 4 }
{ mfr = "WALL HERO", model = "ACL-401S8I", switches = 4, buttons = 4 },
{ mfr = "WALL HERO", model = "ACL-401S3I", switches = 3, buttons = 0 }
Copy link
Contributor

@liuyd96 liuyd96 Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a single commit point of view, your code has no problems.
And It’s okay if you don’t add a comma at the end of the line.

But this may introduce problems when the maintainer merges #981.
In that case, the codes would like

local FINGERPRINTS = {
  { mfr = "WALL HERO", model = "ACL-401S4I", switches = 4, buttons = 0 },
  { mfr = "WALL HERO", model = "ACL-401S8I", switches = 4, buttons = 4 },
  { mfr = "WALL HERO", model = "ACL-401S2I", switches = 2, buttons = 0 } -- Syntax Error
  { mfr = "WALL HERO", model = "ACL-401S3I", switches = 3, buttons = 0 }
}

@greens
Copy link
Contributor

greens commented Oct 18, 2023

branch should also be rebased here, and yes @liuyd96 is correct: these three fingerprint additions will create conflicts that require a rebase after each is merged

@github-actions
Copy link

github-actions bot commented Oct 18, 2023 •
edited
Loading

Channel deleted.

@github-actions
Copy link

github-actions bot commented Oct 18, 2023 •
edited
Loading

Test Results

     54 files     345 suites   0s ⏱️
1 622 tests 1 622 ✔️ 0 💤 0 ❌
2 842 runs  2 842 ✔️ 0 💤 0 ❌

Results for commit c75eafb.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 18, 2023 •
edited
Loading

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 62111a3

@lelandblue lelandblue requested a review from wkhenon October 18, 2023 19:05
@liuyd96
Copy link
Contributor

liuyd96 commented Oct 19, 2023 •
edited
Loading

Hi, @thinkaName

Please refer to below steps to rebase your branch.

(Now I am using your s3i branch as an example)

  1. Add official repo (If you have already added it, please skip this step )
 SmartThingsEdgeDrivers on  vensi-jian/s3i
❯ git remote add upstream https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers.git
  1. Do git rebase
    As you can see there are some conflicts that needed to be solved.
in SmartThingsEdgeDrivers on  vensi-jian/s3i
❯ git pull --rebase upstream main
From https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers
 * branch            main       -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: wallhero add model ACL-401S3I
Using index info to reconstruct a base tree...
M       drivers/SmartThings/zigbee-switch/fingerprints.yml
Falling back to patching base and 3-way merge...
Auto-merging drivers/SmartThings/zigbee-switch/fingerprints.yml
CONFLICT (content): Merge conflict in drivers/SmartThings/zigbee-switch/fingerprints.yml
error: Failed to merge in the changes.
Patch failed at 0001 wallhero add model ACL-401S3I
hint: Use 'git am --show-current-patch' to see the failed patch
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
  1. Fix Conflicts
  • Show Conflicts
in SmartThingsEdgeDrivers (63214e1) (REBASING 1/2) [=+]
❯ git diff
diff --cc drivers/SmartThings/zigbee-switch/fingerprints.yml
index e12d675,30f1098..0000000
--- a/drivers/SmartThings/zigbee-switch/fingerprints.yml
+++ b/drivers/SmartThings/zigbee-switch/fingerprints.yml
@@@ -2146,10 -2146,10 +2146,17 @@@ zigbeeManufacturer
      manufacturer: WALL HERO
      model: ACL-401S8I
      deviceProfileName: basic-switch
++<<<<<<< HEAD
 +  - id: "Insta GmbH/Switching Actuator"
 +    deviceLabel: NEXENTRO Switching Actuator
 +    manufacturer: Insta GmbH
 +    model: NEXENTRO Switching Actuator
++=======
+   - id: "WALL HERO/ACL-401S3I"
+     deviceLabel: 三位智能开关面板 1
+     manufacturer: WALL HERO
+     model: ACL-401S3I
++>>>>>>> wallhero add model ACL-401S3I
      deviceProfileName: basic-switch
    - id: "Insta GmbH/Dimming Actuator"
      deviceLabel: NEXENTRO Dimming Actuator
  • Fix conflicts
in SmartThingsEdgeDrivers (63214e1) (REBASING 1/2) [=+] took 21s
❯ git diff
diff --cc drivers/SmartThings/zigbee-switch/fingerprints.yml
index e12d675,30f1098..0000000
--- a/drivers/SmartThings/zigbee-switch/fingerprints.yml
+++ b/drivers/SmartThings/zigbee-switch/fingerprints.yml
@@@ -2146,11 -2146,11 +2146,16 @@@ zigbeeManufacturer
      manufacturer: WALL HERO
      model: ACL-401S8I
      deviceProfileName: basic-switch
 +  - id: "Insta GmbH/Switching Actuator"
 +    deviceLabel: NEXENTRO Switching Actuator
 +    manufacturer: Insta GmbH
 +    model: NEXENTRO Switching Actuator
 +    deviceProfileName: basic-switch
+   - id: "WALL HERO/ACL-401S3I"
+     deviceLabel: 三位智能开关面板 1
+     manufacturer: WALL HERO
+     model: ACL-401S3I
+     deviceProfileName: basic-switch
    - id: "Insta GmbH/Dimming Actuator"
      deviceLabel: NEXENTRO Dimming Actuator
      manufacturer: Insta GmbH
  • Add file
in SmartThingsEdgeDrivers (63214e1) (REBASING 1/2) [=+]
❯ git add drivers/SmartThings/zigbee-switch/fingerprints.yml
  1. Continue rebase
in SmartThingsEdgeDrivers (63214e1) (REBASING 1/2) [+]
❯ git rebase --continue
Applying: wallhero add model ACL-401S3I
Applying: Update init.lua

Now everything is done. You could see there is no merge commit in changelog.

in SmartThingsEdgeDrivers on  vensi-jian/s3i
❯ git log --oneline
0a66887 (HEAD -> vensi-jian/s3i) Update init.lua
9010fd7 wallhero add model ACL-401S3I
63214e1 (upstream/main) Merge pull request #926 from SmartThingsCommunity/fix/hue-poll-connectivity-on-refresh

I recommend to use git rebase instead of git merge when developing git projects : )
And remember to do git rebase regularly : ）

@lelandblue lelandblue merged commit 0249339 into SmartThingsCommunity:main Nov 9, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liuyd96 liuyd96 liuyd96 left review comments

@greens greens greens approved these changes

@mtilchen mtilchen Awaiting requested review from mtilchen

@cbaumler cbaumler Awaiting requested review from cbaumler

@wkhenon wkhenon Awaiting requested review from wkhenon

Assignees
No one assigned
Labels
WWST
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thinkaName @greens @liuyd96 @lelandblue